[Security Solution][Detections][Threshold Rules] Threshold multiple aggregations with cardinality #90826

madirey · 2021-02-09T18:26:27Z

Summary

This PR adds support for multiple terms aggregations within a Threshold Rule, as well as an additional cardinality aggregation for matching a specific number of unique values across a field.

Pre-existing signals from 7.11.x are handled without a migration by converting the existing document to a 7.12-conforming document DE-side and client-side before evaluating.

TODO:

~~Add migration for threshold and threshold_result or handle pre-7.12 schema separately~~
https://github.com/elastic/security-team/issues/839

Checklist

Delete any items that are not applicable to this PR.

Unit or functional tests were updated or added to match the most common scenarios

For maintainers

This was checked for breaking API changes and was labeled appropriately

…tiple-aggs

This reverts commit b73b34a.

…tiple-aggs

...lugins/security_solution/server/lib/detection_engine/signals/threshold_get_bucket_filters.ts

x-pack/plugins/security_solution/public/detections/components/rules/threshold_input/index.tsx

...lugins/security_solution/server/lib/detection_engine/signals/threshold_get_bucket_filters.ts

x-pack/plugins/security_solution/public/detections/components/alerts_table/actions.tsx

...ugins/security_solution/server/lib/detection_engine/signals/bulk_create_threshold_signals.ts

...plugins/security_solution/server/search_strategy/timeline/factory/events/all/helpers.test.ts

…tiple-aggs

This reverts commit 3edc7f2.

lykkin

🎉 🦖 📦 🐔

lykkin · 2021-02-17T20:15:01Z

...plugins/security_solution/common/search_strategy/security_solution/matrix_histogram/index.ts

@@ -36,7 +36,14 @@ export interface MatrixHistogramRequestOptions extends RequestBasicOptions {
  timerange: TimerangeInput;
  histogramType: MatrixHistogramType;
  stackByField: string;
-  threshold?: { field: string | undefined; value: number } | undefined;
+  threshold?:


looks like a few changes similar to this one were made in this PR, would it be worth pulling this out into its own type shared between the updated code? not a blocker, but might be a nice follow up.

marshallmain · 2021-02-17T17:48:15Z

x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_bulk_body.test.ts

+          threshold: {
+            field: ['host.name'],
+            value: 100,
+          },


this rule has type query so adding threshold fields to it makes an invalid rule. The test works for now, but may be a pain later if build_bulk_body starts using the stricter types that keep different rule types separated.

marshallmain · 2021-02-17T17:51:13Z

x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_signal.ts

@@ -95,16 +96,24 @@ export const buildSignal = (docs: BaseSignalHit[], rule: RulesSchema): Signal =>
  };
 };

+const isThresholdResult = (thresholdResult: SearchTypes): thresholdResult is ThresholdResult => {
+  return typeof thresholdResult === 'object';


any array will satisfy this check

marshallmain · 2021-02-17T19:54:21Z

...lugins/security_solution/server/lib/detection_engine/signals/threshold_get_bucket_filters.ts

+          field,
+          value: result[0].value,
+        };
+      });


One edge case to consider here is what happens when a threshold rule is edited and the list of fields to aggregate on changes.

Suppose threshold.fields was ["source.ip", "destination.ip", host.name"] and we edit the rule removing host.name. On the next rule run, when we find a signal that was made pre-edit and build a filter based on that signal the filter will only include source.ip and destination.ip since host.name will no longer be in bucketByFields. This filter will be broader than the bucket from the original signal so we risk excluding documents that were not part of the original set that met the threshold.

Instead we could create the filters based only on signalTerms and not use bucketByFields at all when generating these filters. This way, in the above case where a field is removed from threshold rule, future rule runs will still include that field and value in the filter.

I think the current behavior is good enough that these edge cases can be addressed in a follow up PR.

…tiple-aggs

marshallmain · 2021-02-17T22:57:25Z

Also for a follow up PR: if you specify a cardinality field but leave the "group by" field blank then the cardinality field is silently ignored. We should either disallow this combination or use the cardinality field when "group by" is blank.

…tiple-aggs

…ggregations with cardinality (#90826) (#91792) * Remove unnecessary spreads * Layout, round 1 * Revert "Layout, round 1" This reverts commit b73b34a. * Make threshold field an array * Add cardinality fields * Fix validation schema * Query for multi-aggs * Finish multi-agg aggregation * Translate to multi-agg buckets * Fix existing tests and add new test skeletons * clean up * Fix types * Fix threshold_result data structure * previous signals filter * Fix previous signal detection * Finish previous signal parsing * tying up loose ends * Fix timeline view for multi-agg threshold signals * Fix build_bulk_body tests * test fixes * Add test for threshold bucket filters * Address comments * Fixing schema errors * Remove unnecessary comment * Fix tests * Fix types * linting * linting * Fixes * Handle pre-7.12 threshold format in timeline view * missing null check * adding in follow-up pr * Handle pre-7.12 filters * unnecessary change * Revert "unnecessary change" This reverts commit 3edc7f2. * linting * Fix rule schemas * Fix tests Co-authored-by: Marshall Main <marshall.main@elastic.co> Co-authored-by: Marshall Main <marshall.main@elastic.co>

kibanamachine · 2021-02-18T06:27:09Z

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`securitySolution`	7.7MB	7.7MB	+5.7KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`securitySolution`	238.5KB	239.3KB	+790.0B

History

💔 Build #107554 failed b6fd98b
💚 Build #107558 succeeded 5c503fc
💔 Build #107549 failed 9ba09b6
💔 Build #107507 failed db6dfa5
💔 Build #107489 failed 6eafa8d

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@madirey

… Rules (#522) This is a rough pass at updating the [Create Rule API docs](https://www.elastic.co/guide/en/security/current/rules-api-create.html#_request_body) for the recent changes surrounding `Threshold` and `Indicator Match` rules. @madirey and @rylnd if you could verify these changes and if any additional fields should be documented that would be greatly appreciated. 🙂 Resolves: elastic/kibana#91965 Note: The grouping of optional fields per rule type makes these docs difficult to maintain (for me at least 😅), as you need to cross reference each optional field from each type, plus I'd bargain our users might find it useful to be able to look at all fields per each rule type independently. Something to verify with users/discuss, but would be a nice add for maintaining these docs (outside of writing scripts to generate the groupings). ##### Threshold Rules: For more details on `cardinality_field` and `cardinality_value` see: elastic/kibana#90826 ##### Indicator Match Rules: For more details on `threat_filters` see: elastic/kibana#91260

@madirey

… Rules (elastic#522) This is a rough pass at updating the [Create Rule API docs](https://www.elastic.co/guide/en/security/current/rules-api-create.html#_request_body) for the recent changes surrounding `Threshold` and `Indicator Match` rules. @madirey and @rylnd if you could verify these changes and if any additional fields should be documented that would be greatly appreciated. 🙂 Resolves: elastic/kibana#91965 Note: The grouping of optional fields per rule type makes these docs difficult to maintain (for me at least 😅), as you need to cross reference each optional field from each type, plus I'd bargain our users might find it useful to be able to look at all fields per each rule type independently. Something to verify with users/discuss, but would be a nice add for maintaining these docs (outside of writing scripts to generate the groupings). ##### Threshold Rules: For more details on `cardinality_field` and `cardinality_value` see: elastic/kibana#90826 ##### Indicator Match Rules: For more details on `threat_filters` see: elastic/kibana#91260

@madirey

… Rules (elastic#522) This is a rough pass at updating the [Create Rule API docs](https://www.elastic.co/guide/en/security/current/rules-api-create.html#_request_body) for the recent changes surrounding `Threshold` and `Indicator Match` rules. @madirey and @rylnd if you could verify these changes and if any additional fields should be documented that would be greatly appreciated. 🙂 Resolves: elastic/kibana#91965 Note: The grouping of optional fields per rule type makes these docs difficult to maintain (for me at least 😅), as you need to cross reference each optional field from each type, plus I'd bargain our users might find it useful to be able to look at all fields per each rule type independently. Something to verify with users/discuss, but would be a nice add for maintaining these docs (outside of writing scripts to generate the groupings). ##### Threshold Rules: For more details on `cardinality_field` and `cardinality_value` see: elastic/kibana#90826 ##### Indicator Match Rules: For more details on `threat_filters` see: elastic/kibana#91260

@madirey

… Rules (#522) (#540) This is a rough pass at updating the [Create Rule API docs](https://www.elastic.co/guide/en/security/current/rules-api-create.html#_request_body) for the recent changes surrounding `Threshold` and `Indicator Match` rules. @madirey and @rylnd if you could verify these changes and if any additional fields should be documented that would be greatly appreciated. 🙂 Resolves: elastic/kibana#91965 Note: The grouping of optional fields per rule type makes these docs difficult to maintain (for me at least 😅), as you need to cross reference each optional field from each type, plus I'd bargain our users might find it useful to be able to look at all fields per each rule type independently. Something to verify with users/discuss, but would be a nice add for maintaining these docs (outside of writing scripts to generate the groupings). ##### Threshold Rules: For more details on `cardinality_field` and `cardinality_value` see: elastic/kibana#90826 ##### Indicator Match Rules: For more details on `threat_filters` see: elastic/kibana#91260

@madirey

… Rules (#522) (#539) This is a rough pass at updating the [Create Rule API docs](https://www.elastic.co/guide/en/security/current/rules-api-create.html#_request_body) for the recent changes surrounding `Threshold` and `Indicator Match` rules. @madirey and @rylnd if you could verify these changes and if any additional fields should be documented that would be greatly appreciated. 🙂 Resolves: elastic/kibana#91965 Note: The grouping of optional fields per rule type makes these docs difficult to maintain (for me at least 😅), as you need to cross reference each optional field from each type, plus I'd bargain our users might find it useful to be able to look at all fields per each rule type independently. Something to verify with users/discuss, but would be a nice add for maintaining these docs (outside of writing scripts to generate the groupings). ##### Threshold Rules: For more details on `cardinality_field` and `cardinality_value` see: elastic/kibana#90826 ##### Indicator Match Rules: For more details on `threat_filters` see: elastic/kibana#91260

madirey and others added 26 commits January 14, 2021 15:31

Remove unnecessary spreads

a4139ce

Layout, round 1

b73b34a

Merge branch 'master' of github.com:elastic/kibana into threshold-mul…

026a6d7

…tiple-aggs

Merge branch 'master' of github.com:elastic/kibana into threshold-mul…

8c5c889

…tiple-aggs

Revert "Layout, round 1"

ed5f9b0

This reverts commit b73b34a.

Make threshold field an array

b2f31ba

Add cardinality fields

0842e84

Fix validation schema

02c3b2c

Query for multi-aggs

48f6545

Merge branch 'master' of github.com:elastic/kibana into threshold-mul…

e435241

…tiple-aggs

Finish multi-agg aggregation

70e4f50

Translate to multi-agg buckets

cf7ef93

Fix existing tests and add new test skeletons

1ebfed0

merge master, fix conflicts

0786757

clean up

12a98bb

Fix types

6cba63a

Fix threshold_result data structure

78e77bc

previous signals filter

465c5a4

Fix previous signal detection

18d5363

Finish previous signal parsing

41a5ddb

tying up loose ends

19ed253

merge master, fix conflicts

733347c

Merge branch 'master' of github.com:elastic/kibana into threshold-mul…

c7eea31

…tiple-aggs

Fix timeline view for multi-agg threshold signals

319e9db

Fix build_bulk_body tests

e2a7d40

test fixes

c6abdf5

marshallmain reviewed Feb 16, 2021

View reviewed changes

madirey added 3 commits February 16, 2021 17:56

Add test for threshold bucket filters

741c75e

Address comments

b277b04

Fixing schema errors

8d1e922

madirey added 7 commits February 17, 2021 16:54

adding in follow-up pr

bed1faf

Handle pre-7.12 filters

e979d12

Merge branch 'master' of github.com:elastic/kibana into threshold-mul…

9dab01e

…tiple-aggs

unnecessary change

3edc7f2

Revert "unnecessary change"

13821bf

This reverts commit 3edc7f2.

linting

f88cf66

Fix rule schemas

3e09b24

lykkin approved these changes Feb 17, 2021

View reviewed changes

marshallmain approved these changes Feb 17, 2021

View reviewed changes

madirey added 2 commits February 17, 2021 22:05

Merge branch 'master' of github.com:elastic/kibana into threshold-mul…

6eafa8d

…tiple-aggs

Fix tests

db6dfa5

Merge branch 'master' of github.com:elastic/kibana into threshold-mul…

9ba09b6

…tiple-aggs

madirey enabled auto-merge (squash) February 18, 2021 01:18

madirey added 2 commits February 18, 2021 01:41

merge master, fix conflicts

b6fd98b

more fixing conflicts

5c503fc

madirey merged commit 5b0e283 into elastic:master Feb 18, 2021

madirey deleted the threshold-multiple-aggs branch February 18, 2021 04:08

madirey mentioned this pull request Feb 18, 2021

[7.x] [Security Solution][Detections][Threshold Rules] Threshold multiple aggregations with cardinality (#90826) #91792

Merged

spong mentioned this pull request Feb 19, 2021

[DOCS] Updates Create Rule API docs for Threshold and Indicator Match Rules elastic/security-docs#522

Merged

spong added the documentation label Mar 1, 2021

jmikell821 mentioned this pull request Mar 4, 2021

[DOCS] Threshold rules: multiple aggregations with cardinality elastic/security-docs#535

Closed

peluja1012 mentioned this pull request Jun 17, 2021

[Security Solution][Detections] Ability to group by multiple fields for Threshold detection rule #86100

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution][Detections][Threshold Rules] Threshold multiple aggregations with cardinality #90826

[Security Solution][Detections][Threshold Rules] Threshold multiple aggregations with cardinality #90826

madirey commented Feb 9, 2021 •

edited

Loading

lykkin left a comment

lykkin Feb 17, 2021

marshallmain Feb 17, 2021

marshallmain Feb 17, 2021

marshallmain Feb 17, 2021

marshallmain commented Feb 17, 2021

kibanamachine commented Feb 18, 2021

[Security Solution][Detections][Threshold Rules] Threshold multiple aggregations with cardinality #90826

[Security Solution][Detections][Threshold Rules] Threshold multiple aggregations with cardinality #90826

Conversation

madirey commented Feb 9, 2021 • edited Loading

Summary

Checklist

For maintainers

lykkin left a comment

Choose a reason for hiding this comment

lykkin Feb 17, 2021

Choose a reason for hiding this comment

marshallmain Feb 17, 2021

Choose a reason for hiding this comment

marshallmain Feb 17, 2021

Choose a reason for hiding this comment

marshallmain Feb 17, 2021

Choose a reason for hiding this comment

marshallmain commented Feb 17, 2021

kibanamachine commented Feb 18, 2021

💚 Build Succeeded

Metrics [docs]

Async chunks

Page load bundle

History

madirey commented Feb 9, 2021 •

edited

Loading